Java Assignment3 upload by JunHoMun#34
Open
gromit-95 wants to merge 3 commits intoFastCampusKDTBackend:mainfrom
Open
Java Assignment3 upload by JunHoMun#34gromit-95 wants to merge 3 commits intoFastCampusKDTBackend:mainfrom
gromit-95 wants to merge 3 commits intoFastCampusKDTBackend:mainfrom
Conversation
MinChul-Son
reviewed
Apr 25, 2023
MinChul-Son
left a comment
MinChul-Son
left a comment
There was a problem hiding this comment.
안녕하세요 준호님~~
과제 진행하신다고 고생많으셨습니다!!
코드를 작성하실 때는 자바에서 권장하는 코드 스타일에 맞게 코드를 작성해주시면 좋을 것 같아요!
Google Code Style Guide를 참고해보시죠!
깃 커밋을 조금 더 세분화해서 작업의 단위를 잘게 쪼개보는 것도 좋을 것 같아요.
그리고 pr을 올리실 때 본인만의 템플릿을 가지고 계시면 좋을 것 같습니다.
ex)
- 내가 중점적으로 생각한 부분
- 작업 내용
- 궁금한 점
- 함께 이야기하고 싶은 부분
과 같은 것들이 포함되면 좋습니다!
대부분이 중복된 코멘트라 최대한 줄였어요~! 이해가 되지 않는 부분은 편하게 이야기해주세요.
|
|
||
| public class Electronic { | ||
|
|
||
| public enum companyName {SAMSUNG,LG,APPLE} |
There was a problem hiding this comment.
Suggested change
| public enum companyName {SAMSUNG,LG,APPLE} | |
| public enum CompanyName {SAMSUNG,LG,APPLE} |
| private authMethod[] authMethods; | ||
| private companyName company; | ||
| private String productNo; | ||
| private static int productionCount=1; |
There was a problem hiding this comment.
static 변수는 일반 멤버 변수보다 위로 위치시켜주는 것이 관례입니다😀
| private authMethod[] authMethods; | ||
| private companyName company; | ||
| private String productNo; | ||
| private static int productionCount=1; |
There was a problem hiding this comment.
Suggested change
| private static int productionCount=1; | |
| private static int productionCount = 1; |
사소한거지만 코드 스타일도 맞춰주시면 좋을 것 같아요!! 인텔리제이의 자동정렬 기능을 활용해보시죠~
| public class Electronic { | ||
|
|
||
| public enum companyName {SAMSUNG,LG,APPLE} | ||
| public enum authMethod {FINGERPRINT, PATTERN, PIN, FACE} |
There was a problem hiding this comment.
Suggested change
| public enum authMethod {FINGERPRINT, PATTERN, PIN, FACE} | |
| public enum AuthMethod {FINGERPRINT, PATTERN, PIN, FACE} |
|
|
||
| StringBuffer generateProductNo; | ||
| generateProductNo = new StringBuffer(LocalDate.now().getYear()%100); | ||
| generateProductNo.append(String.format("%02",LocalDate.now().getDayOfMonth())); |
| return null; | ||
| } | ||
|
|
||
| public User copy(User user) {// Deep copy |
| private static final int DEFAULT =10; | ||
| private int size; | ||
| private int capacity; | ||
| private Electronics(){ |
There was a problem hiding this comment.
Suggested change
| private Electronics(){ | |
| private Electronics(){ |
| public class Electronics { | ||
|
|
||
| private static Electronics allElectronics; | ||
| public Electronics getInstance(){ |
| size = 0; | ||
| } | ||
|
|
||
| public static Electronics getAllElectronics() { |
There was a problem hiding this comment.
getInstance만을 통해 객체 인스턴스를 제공받을 수 있게하면 좋을 것 같아요.
이 메서드는 어디서 사용되나요?
| return allElectronics; | ||
| } | ||
|
|
||
| public static void setAllElectronics(Electronics allElectronics) { |
MinChul-Son
reviewed
Apr 26, 2023
| Electronic[] groupByAuthMethodArray= new Electronic[size]; | ||
| int j=0; | ||
| for(int i=0;i<size;i++){ | ||
| if(electronicList[i].getCompany().equals(authMethod.name())){ |
There was a problem hiding this comment.
getCompany()가 아니라 getAuthMethod()가 되어야할것 같고 반환형이 AuthMethod[]라서 코드가 바뀌어야할 것 같네요~
MinChul-Son
reviewed
Apr 26, 2023
| newUser.setUserPhoneNumber(user.getUserPhoneNumber()); | ||
| newUser.setUserEmail(user.getUserEmail()); | ||
| newUser.setUserBirthDate(user.getUserBirthDate()); | ||
| newUser.setElectronicDevices(user.getElectronicDevices()); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
요번 과제 많이 어렵네요... 코드리뷰 부탁드립니다... ㅠ